Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve UX querying rpc for blocks at or before the snapshot from boot #23403

Merged
merged 3 commits into from
Mar 1, 2022

Conversation

CriesofCarrots
Copy link
Contributor

Problem

When an RPC node not backed by bigtable boots from a snapshot and fresh ledger, it initially reports null for getBlock requests for all slots <= snapshot slot + 1. This is because:

  1. For the case of slots <= snapshot_slot: When a node starts from a snapshot and empty ledger, the Blockstore::lowest_cleanup_slot reports 0. This is the field that RPC checks for the RpcCustomError::BlockCleanedUp error, so it doesn't know that the snapshot slot and older slots aren't available. This persists until the first ledger prune occurs.
  2. For the case of slot == snapshot_slot +1: RPC now requires that blocks queried from Blockstore be complete, including the parent_blockhash. Parent_blockhashes are figured from slot entries, so this value isn't available for the snapshot slot.

Summary of Changes

  • Update Blockstore::get_first_available_block to be root at index 1, instead of index 0, so that the value reflects the first completely available block
  • Use get_first_available_block in JsonRpcRequestProcessor::check_slot_cleaned_up() and return RpcCustomError::BlockCleanedUp when older blocks are queried. This new case needs to cover both Err and Ok cases to handle Blockstore::get_complete_block and Blockstore::get_block_time responses, respectively. Regarding the error message: technically these blocks haven't been cleaned up... they never existed on this node. But this seems most consistent with the spirit of the error to me, and less confusing than it would be to have the error message change after the first prune.

Note: these changes do not affect nodes with bigtable backing, since all methods query bigtable and return early before reaching check_slot_cleaned_up()

@CriesofCarrots CriesofCarrots requested a review from t-nelson March 1, 2022 03:13
Copy link
Contributor

@t-nelson t-nelson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice and clean! lgtm

@CriesofCarrots CriesofCarrots merged commit 3b5b71c into solana-labs:master Mar 1, 2022
mergify bot pushed a commit that referenced this pull request Mar 1, 2022
#23403)

* Bump first-available block to first complete block

* Remove obsolete purges in tests (PrimaryIndex toggling no longer in use

* Check first-available block in Rpc check_slot_cleaned_up

(cherry picked from commit 3b5b71c)

# Conflicts:
#	ledger/src/blockstore.rs
jeffwashington pushed a commit to jeffwashington/solana that referenced this pull request Mar 1, 2022
solana-labs#23403)

* Bump first-available block to first complete block

* Remove obsolete purges in tests (PrimaryIndex toggling no longer in use

* Check first-available block in Rpc check_slot_cleaned_up
mergify bot added a commit that referenced this pull request Mar 1, 2022
…t (backport #23403) (#23405)

* Improve UX querying rpc for blocks at or before the snapshot from boot (#23403)

* Bump first-available block to first complete block

* Remove obsolete purges in tests (PrimaryIndex toggling no longer in use

* Check first-available block in Rpc check_slot_cleaned_up

(cherry picked from commit 3b5b71c)

# Conflicts:
#	ledger/src/blockstore.rs

* Fix conflicts

Co-authored-by: Tyera Eulberg <[email protected]>
jeffwashington pushed a commit to jeffwashington/solana that referenced this pull request Mar 4, 2022
solana-labs#23403)

* Bump first-available block to first complete block

* Remove obsolete purges in tests (PrimaryIndex toggling no longer in use

* Check first-available block in Rpc check_slot_cleaned_up
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants